fix(contrib/trivy): dedup package names and add dup warnings#2491
fix(contrib/trivy): dedup package names and add dup warnings#2491
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Trivy JSON-to-Vuls conversion logic to deterministically deduplicate OS packages (notably for Debian/Ubuntu dpkg where Trivy can emit duplicates) by keeping the newer version using dpkg-aware comparison, and adds unit tests covering the duplicate scenarios.
Changes:
- Add
versionGreaterThan()to compare Debian/Ubuntu versions using dpkg semantics (with a lexicographic fallback). - Change OS package (
Packages/SrcPackages) aggregation to keep the newest version while accumulatingBinaryNamesfor source packages. - Add
TestConverttable tests covering duplicate ordering and non-dpkg fallback behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| contrib/trivy/pkg/converter.go | Deduplicate packages by version comparison (dpkg-aware for Debian/Ubuntu) and accumulate source BinaryNames. |
| contrib/trivy/pkg/converter_test.go | Add conversion tests to validate de-duplication and source/binary aggregation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shino
force-pushed
the
shino/fix-pkg-dup
branch
from
88c609d to
ed89ab3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Ooops, detected vulns by trivy remains after trivy-to-vuls... 🤷 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shino
changed the title
| dupPkgs[p.Name] = append(versions, pv) | ||
| } | ||
| } | ||
| if compareVersions(trivyResult.Type, pv, existing.Version) >= 0 { |
There was a problem hiding this comment.
nit: compareVersions(...) >= 0 means the map entry is overwritten even when the two versions are equal. Changing the condition to > 0 would skip the redundant reassignment and make the intent clearer — we only want to replace when the new version is strictly newer.
| if compareVersions(trivyResult.Type, pv, existing.Version) >= 0 { | |
| if compareVersions(trivyResult.Type, pv, existing.Version) > 0 { |
There was a problem hiding this comment.
This is tricky part (maybe I should have written comment). 🤔
|
|
||
| if existing, ok := srcPkgs[p.SrcName]; ok { | ||
| existing.AddBinaryName(p.Name) | ||
| if compareVersions(trivyResult.Type, sv, existing.Version) >= 0 { |
There was a problem hiding this comment.
nit: compareVersions(...) >= 0 means the map entry is overwritten even when the two versions are equal. Changing the condition to > 0 would skip the redundant reassignment and make the intent clearer — we only want to replace when the new version is strictly newer.
| if compareVersions(trivyResult.Type, sv, existing.Version) >= 0 { | |
| if compareVersions(trivyResult.Type, sv, existing.Version) > 0 { |
There was a problem hiding this comment.
Strictly speaking, = does not needed here.
I added it to make bin/src collection seems as same as the possible.
But it can be removed. It may be a matter of taste? Which do you prefer?
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
shino
force-pushed
the
shino/fix-pkg-dup
branch
from
d3e5edb to
2c4d940
Compare
Problem
On Debian/Ubuntu, Trivy's dpkg analyzer reads both status and
/var/lib/dpkg/status.d/*. Because the applier's dedup key includesFilePath, entries from different paths survive deduplication and appear as duplicate packages in Trivy's JSON output.The previous converter code was also inconsistent in its handling of duplicates:
Packages(binary) used last-wins (unconditional overwrite), whileSrcPackagesused first-wins (only the first occurrence locked in the version, subsequent duplicates only accumulatedBinaryNames). Combined with Trivy'ssort.Sort— which sorts packages lexicographically by(Name, Version), not by semantic version — this meant the result depended on input order and string sort, silently dropping the correct version in some cases.This led to false-positive vulnerability detections — for example, a host running
openssl 3.5.5-1~deb13u1(patched) would be reported as running3.5.4-1~deb13u1(vulnerable) because the stale entry fromstatus.d/was picked up by the first-winsSrcPackageslogic.Fix
Replaced the inconsistent first-wins / last-wins with explicit version comparison that always keeps the newer version:
go-deb-version(github.com/knqyf263/go-deb-version, already a dependency) for proper dpkg version semantics. The newer version always wins regardless of input order.The helper function
compareVersions(osType, a, b) intdispatches between dpkg and lexicographic comparison, returning a positive value ifa > b, zero if equal, and a negative value ifa < b. If dpkg version parsing fails for either operand, it falls back to lexicographic comparison.For
SrcPackages,BinaryNamesare accumulated across all duplicates before the version comparison, preserving the complete list of binary packages.When duplicate packages are detected, a per-package warning is emitted via
ScanResult.Warningslisting the package name and all versions seen.Changes
contrib/trivy/pkg/converter.go:compareVersions()helper (returnsint) with dpkg vs. lexicographic dispatchpkgsdedup: only overwrite when new entry has a newer or equal version (first occurrence always kept)srcPkgsdedup: same logic, withBinaryNamesalways accumulatedScanResult.Warningscontrib/trivy/pkg/converter_test.go:TestConvertwith 5 test cases:Limitations
This PR deduplicates
PackagesandSrcPackagesby always keeping the newest version, making the converter output deterministic and correct for package metadata. However, it does not address the following issues that stem from Trivy itself emitting duplicate packages:False-positive vulnerabilities remain in
ScannedCves. When Trivy detects a CVE against the stale (older) duplicate package fromstatus.d/, that vulnerability entry is passed through to the converter as-is. The converter does not filter vulnerabilities, so the false-positive CVE will still appear in the output.AffectedPackagesmay reference versions not present inPackagesorSrcPackages. Because the converter keeps only the newest version inPackages/SrcPackagesbut preserves all vulnerability entries from Trivy, a CVE'sAffectedPackages.FixedInmay refer to a version older than the installed version — a version that no longer exists in the package maps. This can result in an inconsistent state where a CVE appears to affect a package whose installed version already exceeds the fix version.The root cause is that Trivy's dpkg analyzer reads both
statusandstatus.d/without deduplication. The recommended fix for affected users is to remove/var/lib/dpkg/status.d/from their container images whenstatusis present, asstatus.d/is intended only for distroless images that lack a full dpkg installation.